adopt Ava default file glob #8653
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
turadg
force-pushed
the
ta/migrate-test-names
branch
2 times, most recently
from
45c6d7e to
d86551c
Compare
turadg
force-pushed
the
ta/migrate-test-names
branch
from
d86551c to
588007c
Compare
erights
approved these changes
Dec 23, 2023
There was a problem hiding this comment.
With
Once this is in SDK, we can do the same in other Agoric repos.
in the PR comment, this PR LGTM. However, I'd like to see such a PR actually happen and be approved for endo before seeing both merged. After a month of unanticipated pain bringing our two main repos back in sync, the last thing I want to see is for them to diverge again. This makes it hazardous to merge either until both are approved.
See endojs/endo#1913
This was referenced Dec 23, 2023
This was referenced May 8, 2024
|
I think the script will mis-handle generated |
Good catch. I'll regenerate them and delete the old one. |
turadg
force-pushed
the
ta/migrate-test-names
branch
from
588007c to
8f3160c
Compare
Deploying agoric-sdk with
|
| Latest commit: |
9b8865f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ccc6109c.agoric-sdk.pages.dev |
| Branch Preview URL: | https://ta-migrate-test-names.agoric-sdk.pages.dev |
turadg
force-pushed
the
ta/migrate-test-names
branch
3 times, most recently
from
6ee4f59 to
a8ace47
Compare
turadg
force-pushed
the
ta/migrate-test-names
branch
from
a8ace47 to
9b8865f
Compare
|
@Mergifyio queue |
🟠 Waiting for conditions to match
|
|
@Mergifyio refresh |
✅ Pull request refreshed |
|
@Mergifyio queue |
🛑 Invalid queue nameThe queue |
turadg
added a commit
to endojs/endo
that referenced
this pull request
May 9, 2024
Refs: Agoric/agoric-sdk#8273 ## Description Akin to Agoric/agoric-sdk#8653 ### Security Considerations none ### Scaling Considerations none ### Documentation Considerations reduces need to document differences ### Testing Considerations CI ### Compatibility Considerations Developers will need to know to make the new files. This globs for both old and new so no tests are accidentally omitted from CI. Agoric/agoric-sdk#8273 will be closed by dropping the custom glob and ensuring no old names remain. ### Upgrade Considerations none
mergify bot
added a commit
that referenced
this pull request
May 28, 2024
closes: #8273 ## Description #8653 adopted the new glob but left the old one so any in-flight PRs with new tests wouldn't have them ignored. It's been a month so any should be migrated by now. This removes the old glob. It doesn't remove the `"files"` option entirely, because the default glob is everything under `test` and we have files there that aren't tests. Ava warns they're not tests and and eslint errors when tests import from them. We could go fully to the default glob by using an underscore prefixes on files in `test` that aren't tests. I don't think we need to do that. This change already meets the goal of matching the DX of Ava in other repos. (Test filenames are indicated by a `.test.js` suffix) ### Security Considerations n/a ### Scaling Considerations n/a ### Documentation Considerations reduces need to document ### Testing Considerations no older test files in master (`find packages -name 'test-*' |grep 'test/'`) ### Upgrade Considerations n/a
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes: #8273
Description
Adds a script to migrate a package's file glob to the Ava defaults. See script docs and #8273 for motivation. It uses https://github.com/google/zx because I was banging my head on Bash options and version inconsistencies.
The main thing to review is whether this change is good. Then the code in the first commit. The subsequent commits just are the code running over all the packages. When this PR is approved I'll rebase and re-run the batch to make all the migration commits anew.
Once this is in SDK, we can do the same in other Agoric repos.
Security Considerations
n/a
Scaling Considerations
n/a
Documentation Considerations
If anything, less need for documentation because it aligns with how Ava works by default.
Testing Considerations
To run the script yourself, ensure,
Upgrade Considerations
n/a